Conversation
…control over the models between iterations. Slightly reworked MCMCAcquisition to support this scenario
Codecov Report
@@ Coverage Diff @@
## master #73 +/- ##
=========================================
+ Coverage 99.8% 99.8% +<.01%
=========================================
Files 17 17
Lines 1013 1040 +27
=========================================
+ Hits 1011 1038 +27
Misses 2 2
Continue to review full report at Codecov.
|
| self.counter = 0 | ||
|
|
||
| def __call__(self, models): | ||
| self.counter += 1 |
There was a problem hiding this comment.
Lets think about the callback signature some more. Is there any information we want to pass that might be useful for model building?
For instance, to let the model building strategy depend on the iteration number (we can stop optimizing the hyps after a while like in the MES paper). Although we can also look at the data set size.
What about model building strategies that changes model.X en model.Y (like replace clusters etc.). Not sure if that fits here or is even relevant (the GPflow model should be able to cope with it).
There was a problem hiding this comment.
I think the model contains all the data you need to accomplish something. I believe X and Y can even be updated in this callback as long as the model supports it (all models in GPflow do).
If at some point some information is really missing, this can be added.
|
|
||
| # the call to the constructor of the parent classes, will optimize acquisition, so it obtains the MLE solution. | ||
| super(MCMCAcquistion, self).__init__([acquisition] + copies) | ||
| super(MCMCAcquistion, self).__init__([acquisition]*n_slices) |
There was a problem hiding this comment.
Does this make deep copies? I assumed you used the old way to assure that it were deep copies
There was a problem hiding this comment.
Ah I see, need_new_copies = True makes sure deep copies are made later
There was a problem hiding this comment.
This version does shallow copies, its mostly to assure the copy later on is aware of the amount of copies required without serious overhead.
|
|
||
| # Copy it again if needed due to changed free state | ||
| if self._needs_new_copies: | ||
| new_copies = [copy.deepcopy(self.operands[0]) for _ in range(len(self.operands) - 1)] |
There was a problem hiding this comment.
copy.deepcopy([self.operands[0]]*len(self.operands))
not tested, works too?
There was a problem hiding this comment.
no, the * syntax are shallow copies so the deepcopy will copy the object they are all pointing to.
|
|
||
| def _kill_autoflow(self): | ||
| """ | ||
| Following the recompilation of models, the free state might have changed. This means updating the samples can |
There was a problem hiding this comment.
"""
Flag for recreation on next optimize.
Following the ...
"""
| cause inconsistencies and errors. Flag for recreation on next optimize | ||
| """ | ||
| super(MCMCAcquistion, self)._kill_autoflow() | ||
| self._needs_new_copies = True |
There was a problem hiding this comment.
I assume we cant use needs_setup for this?
There was a problem hiding this comment.
_needs_setup is triggered by a simple set_data. This doesn't require new copies, only in case a callback changes the models (this should happen)
GPflowOpt/bo.py
Outdated
| def jitchol_callback(models): | ||
| """ | ||
| Default callback for BayesianOptimizer. For all GPR models, increase the likelihood variance in case of cholesky | ||
| faillures. This is similar to the use of jitchol in GPy |
There was a problem hiding this comment.
failures
"""
Increase the likelihood ...
This is similar to ... Default callback for BayesianOptimizers. Only usable with GPR models.
"""
| from .pareto import non_dominated_sort | ||
|
|
||
|
|
||
| def jitchol_callback(models): |
There was a problem hiding this comment.
callbacks can be in a separate callbacks.py file?
There was a problem hiding this comment.
I do not plan on shipping any additional callbacks (I might even get rid of this one, it got comitted by accident but it might improve stability?) so that file would be quite empty.
There was a problem hiding this comment.
Ok, I'm not in favor of including jitchol. I think there are other ways users can improve stability. First and foremost putting priors and transforms on the hyps.
There was a problem hiding this comment.
Given #74 I think we should really consider this. For standard scenario's with GPRs (which is what most people will start with) I think this might give an additional automated stability support (which can be disabled by setting the callback to None)
| The acquisition score is computed for each draw, and averaged. | ||
| :param callable callback: (optional) this function or object will be called after each evaluate, after the | ||
| data of all models has been updated with all models as retrieved by acquisition.models as argument without | ||
| the wrapping model handling any scaling . This allows custom model optimization strategies to be implemented. |
There was a problem hiding this comment.
if we do a separate callbacks.py file some of the explanation can be moved there + module link
GPflowOpt/bo.py
Outdated
| initial = initial or EmptyDesign(domain) | ||
| self.set_initial(initial.generate()) | ||
|
|
||
| self._iter_callback = callback |
There was a problem hiding this comment.
why call it iter_callback and not model_callback?
| assert self.acquisition.data[0].shape[1] == newX.shape[-1] | ||
| assert self.acquisition.data[1].shape[1] == newY.shape[-1] | ||
| assert newX.shape[0] == newY.shape[0] | ||
| if newX.size == 0: |
There was a problem hiding this comment.
will this ever happen? As far as I know we cant empty GPflow models so data[0] will never be empty.
There was a problem hiding this comment.
this line avoids _needs_setup = True in case i.e. the EmptyDesign is configured as initial design (as is by default)
There was a problem hiding this comment.
As a sidenote, as GPflow doesn't support models with no data I actually see no use case for BOptimizer having an initial design parameter.
GPflowOpt/bo.py
Outdated
| # If callback specified, and acquisition has the setup flag enabled (indicating an upcoming compilation, | ||
| # run the callback. | ||
| if self._iter_callback and self.acquisition._needs_setup: | ||
| self._iter_callback([m.wrapped for m in self.acquisition.models]) |
There was a problem hiding this comment.
if there is no callback:
- setup is run and models are optimized on the first evaluate
with a callback: - models are optimized here but setup probably has not been run yet and needs_setup is still True -> models are optimized again on first evaluate? right?
There was a problem hiding this comment.
You confuse something here: you can optimize your model in the callback but this is one of the scenarios (which would require optimize_restarts to be 0 in order to avoid two optimizes). The primary use case is to only set the initial starting point.
(The reason the jitchol callback runs the optimization for a small number of steps is to check if no cholesky error occurs, not to optimize the model. )
There was a problem hiding this comment.
Ok, there was indeed some confusion here. I thought the callback would implement the complete model building strategy: setting hyps, running one or more optimizations, etc. This is still possible but you have to set optimize_restarts = 0
gpflowopt/acquisition/acquisition.py
Outdated
| Flag for recreation on next optimize. | ||
|
|
||
| Following the recompilation of models, the free state might have changed. This means updating the samples can | ||
| cause inconsistencies and errors. Flag for recreation on next optimize |
There was a problem hiding this comment.
duplicate "Flag for recreation on next optimize"
gpflowopt/bo.py
Outdated
|
|
||
| def jitchol_callback(models): | ||
| """ | ||
| Increase the likelihood in case of cholesky faillures. |
| jitchol_callback(m.wrapped) # pragma: no cover | ||
|
|
||
| if not isinstance(m, GPR): | ||
| continue |
gpflowopt/bo.py
Outdated
|
|
||
| # Optimization loop | ||
| for i in range(n_iter): | ||
| # If callback specified, and acquisition has the setup flag enabled (indicating an upcoming compilation, |
There was a problem hiding this comment.
If a callback is specified,...
and close brackets :)
Implementation of the final use case defined in #7 . This implements a callback strategy to plug a user defined callable in to BayesianOptimizer which is called each iteration and gives full controls over the models. All GPflow manipulations are possible (assigning priors, modifying transforms, fixing parameters). Goal of those callbacks is to assure optimizations are successful, which can be very application specific.
Combined with the optimize_restarts feature, following use-cases are possible:
This PR depends on #68 and #72 and should be merged after.